chore: update data_substrate#159
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
WalkthroughBumped Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Test as Test Script
participant DB as MySQL / Eloq Store
participant Log as Logger
rect rgba(200,200,255,0.5)
Test->>Log: disable logging
end
rect rgba(200,255,200,0.5)
Test->>DB: execute prepared COUNT(*) (query_get_value)
alt success (no errno)
DB-->>Test: return count
else errno 212 (transient conflict)
DB-->>Test: error 212
Test->>Test: decrement retry counter, sleep 0.5s, retry
loop up to 5 times
Test->>DB: retry COUNT(*)
end
alt exhausted
Test->>Test: abort with fatal error
end
end
end
rect rgba(255,200,200,0.5)
Test->>Log: re-enable logging
Test->>Test: echo final count and query
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
1ca76c6 to
808e6dc
Compare
410cbac to
a15a60e
Compare
a15a60e to
bd46c66
Compare
b40b233 to
7b94dbb
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@storage/eloq/mysql-test/mono_basic/t/range_split_keycache.test`:
- Around line 31-52: The retry loop currently checks query_get_value and then
has separate if blocks that cause the final if ($mysql_errno) to run even when
$mysql_errno == 212; change the flow in the block using $count_retry,
query_get_value($count_stmt,...), $mysql_errno so that the unexpected-error
branch excludes errno 212 (e.g. convert the final "if ($mysql_errno)" to "else
if ($mysql_errno && $mysql_errno != 212)" or simply make it an "else if" after
the 212-handling branch) and apply the same fix to the other retry blocks at
ranges 80-102, 130-152, and 178-200 to prevent immediate abort on retryable
errno 212.
In `@storage/eloq/mysql-test/mono_multi/t/range_split_keycache.test`:
- Around line 33-54: The retry loop treats errno 212 as an unexpected error
because the final "if ($mysql_errno)" branch runs even when $mysql_errno == 212;
change the logic so the unexpected-error branch excludes 212 (e.g., use "else if
($mysql_errno != 212)" or make it an "else if" after the 212-handling block) in
the loop around query_get_value/$count_stmt/$count_retry so the dec $count_retry
retry path for errno 212 can execute; apply the same change to the other retry
blocks referenced at the other ranges (lines ~82-104, ~132-154, ~180-202) that
use $mysql_errno, dec $*_retry and the unexpected-error die.
| while($count_retry) | ||
| { | ||
| let $count_value= query_get_value($count_stmt, count(*), 1); | ||
| if (!$mysql_errno) | ||
| { | ||
| let $count_retry= 0; | ||
| } | ||
| if ($mysql_errno == 212) | ||
| { | ||
| dec $count_retry; | ||
| if (!$count_retry) | ||
| { | ||
| --enable_abort_on_error | ||
| --die "SELECT COUNT(*) failed with read/write conflict after 5 retries"; | ||
| } | ||
| --sleep 0.5 | ||
| } | ||
| if ($mysql_errno) | ||
| { | ||
| --enable_abort_on_error | ||
| --die "SELECT COUNT(*) failed with unexpected error"; | ||
| } |
There was a problem hiding this comment.
Retry loop still aborts immediately on errno 212.
The final if ($mysql_errno) triggers even when $mysql_errno == 212, so the script dies before any retry. Make the “unexpected error” branch exclude 212 (or convert to else if).
🔧 Suggested fix (apply to each retry block)
- if ($mysql_errno)
+ if ($mysql_errno && $mysql_errno != 212)
{
--enable_abort_on_error
--die "SELECT COUNT(*) failed with unexpected error";
}Also applies to: 80-102, 130-152, 178-200
🤖 Prompt for AI Agents
In `@storage/eloq/mysql-test/mono_basic/t/range_split_keycache.test` around lines
31 - 52, The retry loop currently checks query_get_value and then has separate
if blocks that cause the final if ($mysql_errno) to run even when $mysql_errno
== 212; change the flow in the block using $count_retry,
query_get_value($count_stmt,...), $mysql_errno so that the unexpected-error
branch excludes errno 212 (e.g. convert the final "if ($mysql_errno)" to "else
if ($mysql_errno && $mysql_errno != 212)" or simply make it an "else if" after
the 212-handling branch) and apply the same fix to the other retry blocks at
ranges 80-102, 130-152, and 178-200 to prevent immediate abort on retryable
errno 212.
| while($count_retry) | ||
| { | ||
| let $count_value= query_get_value($count_stmt, count(*), 1); | ||
| if (!$mysql_errno) | ||
| { | ||
| let $count_retry= 0; | ||
| } | ||
| if ($mysql_errno == 212) | ||
| { | ||
| dec $count_retry; | ||
| if (!$count_retry) | ||
| { | ||
| --enable_abort_on_error | ||
| --die "SELECT COUNT(*) failed with read/write conflict after 5 retries"; | ||
| } | ||
| --sleep 0.5 | ||
| } | ||
| if ($mysql_errno) | ||
| { | ||
| --enable_abort_on_error | ||
| --die "SELECT COUNT(*) failed with unexpected error"; | ||
| } |
There was a problem hiding this comment.
Retry loop still aborts immediately on errno 212.
The generic error branch fires for 212, so the retry never occurs. Gate the “unexpected error” branch to exclude 212 (or use else if).
🔧 Suggested fix (apply to each retry block)
- if ($mysql_errno)
+ if ($mysql_errno && $mysql_errno != 212)
{
--enable_abort_on_error
--die "SELECT COUNT(*) failed with unexpected error";
}Also applies to: 82-104, 132-154, 180-202
🤖 Prompt for AI Agents
In `@storage/eloq/mysql-test/mono_multi/t/range_split_keycache.test` around lines
33 - 54, The retry loop treats errno 212 as an unexpected error because the
final "if ($mysql_errno)" branch runs even when $mysql_errno == 212; change the
logic so the unexpected-error branch excludes 212 (e.g., use "else if
($mysql_errno != 212)" or make it an "else if" after the 212-handling block) in
the loop around query_get_value/$count_stmt/$count_retry so the dec $count_retry
retry path for errno 212 can execute; apply the same change to the other retry
blocks referenced at the other ranges (lines ~82-104, ~132-154, ~180-202) that
use $mysql_errno, dec $*_retry and the unexpected-error die.
e8e70fa to
fc376a1
Compare
fc376a1 to
449e392
Compare
449e392 to
50e0a3a
Compare
Summary by CodeRabbit